feat(s2n-quic-core): add stream state enums #2132
Conversation
camshaft
force-pushed
the
camshaft/stream-state
branch
from
994d421 to
d08d64e
Compare
camshaft
force-pushed
the
camshaft/stream-state
branch
from
d08d64e to
737397f
Compare
| is!(Recv, is_receiving); | ||
| is!(SizeKnown, is_size_known); | ||
| is!(DataRecvd, is_data_received); | ||
| is!(DataRead, is_data_read); | ||
| is!(ResetRecvd, is_reset_received); | ||
| is!(ResetRead, is_reset_read); | ||
| is!(DataRead | ResetRead, is_terminal); |
There was a problem hiding this comment.
seems like something that would be useful at some point to implement as a derive macro on the Enum
There was a problem hiding this comment.
Yeah possibly. I'm sure there's one that already exists. The question is it worth the dependency 😄
|
|
||
| pub type Result<T> = core::result::Result<(), Error<T>>; | ||
|
|
||
| macro_rules! transition { |
There was a problem hiding this comment.
should this be in a non-stream specific module? it could be used for some of the BBR enums that have state transitions
| pub enum Error<T> { | ||
| NoOp { current: T }, | ||
| InvalidTransition { current: T, target: T }, | ||
| } |
There was a problem hiding this comment.
Ah, so the state transitions for streams would be influenced by the peer, so you'd need to return an error and close the connection (versus just panicking in a debug assertion). This is different than in BBR for example where if a an invalid state transition it was a software bug and we can just panic. So maybe not directly useable by BBR as is, but maybe somewhere else?
There was a problem hiding this comment.
Yeah most likely could be used elsewhere, including BBR where we just have a variant that does debug_assertion instead of return an error.
One nice pattern about returning an error though is you don't have to check the state before transitioning. So something like
fn on_packet_sent(&mut self, packet: &[u8], is_fin: bool) {
// ignore if we're already in this state, just make sure we transition from Ready to Send
let _ = self.state.on_send();
if is_fin && self.state.on_send_fin().is_ok() {
println!("first transmission of fin bit");
}
}
Description of changes:
This change adds a state enum for both Send and Receive. These enums make it easy to ensure only valid state transitions are taken in the actual usage of the code. It also includes some tracing to show when these transitions take place for easy debugging.
Call-outs:
I will switch the current stream implementations to use these state enums as a separate PR.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.